fix(api): isolate side-effect session writes in multimodal and RAG handlers by agarwalpranav0711 · Pull Request #38210 · langgenius/dify

agarwalpranav0711 · 2026-06-30T09:32:06Z

Important

Make sure you have read our contribution guidelines
Ensure there is an associated issue and you have been assigned to it
Use the correct syntax to link this PR: Fixes #<issue number>.

Summary

Extends the session isolation fix from #37895 to two files that were missed: base_app_runner.py and index_tool_callback_handler.py.

Both files used db.session.commit() on the shared Flask request-scoped session to persist side-effect writes (a MessageFile during multimodal LLM streaming, and DatasetQuery audit logs / hit_count updates during RAG retrieval). This flushed all pending ORM changes in the request transaction, not just the intended record, risking premature commits and DetachedInstanceError under concurrency.

Changes:

base_app_runner.py: _handle_multimodal_image_content() now persists the MessageFile through an independent sessionmaker session instead of the shared session.
index_tool_callback_handler.py: on_query() and on_tool_end() now persist audit/analytics writes through an independent session, following the same pattern as fix(api): isolate side-effect database writes #37895.
Added/updated unit tests in both corresponding test files to cover the new session isolation behavior.

Screenshots

N/A - backend transaction/session fix

Checklist

This change requires a documentation update, included: Dify Document
I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
I've updated the documentation accordingly.
I ran make lint && make type-check (backend) and cd web && pnpm exec vp staged (frontend) to appease the lint gods

…ndlers Extends the session isolation fix from langgenius#37895 to two files that were missed: base_app_runner.py and index_tool_callback_handler.py. Both files used db.session.commit() on the shared Flask request-scoped session to persist side-effect writes (a MessageFile during multimodal LLM streaming, and DatasetQuery audit logs / hit_count updates during RAG retrieval). This flushed all pending ORM changes in the request transaction, not just the intended record, risking premature commits and DetachedInstanceError under concurrency. Both now use independent sessionmaker sessions matching the established pattern from langgenius#37895. Fixes langgenius#38176

github-actions · 2026-06-30T13:46:28Z

Pyrefly Diff

base → PR

--- /tmp/pyrefly_base.txt	2026-07-03 07:09:49.851446131 +0000
+++ /tmp/pyrefly_pr.txt	2026-07-03 07:09:34.585416161 +0000
@@ -4261,17 +4261,17 @@
 ERROR Object of class `NoneType` has no attribute `status` [missing-attribute]
    --> tests/unit_tests/core/base/test_app_generator_tts_publisher.py:188:16
 ERROR Argument `None` is not assignable to parameter `app_id` with type `str` in function `core.callback_handler.index_tool_callback_handler.DatasetIndexToolCallbackHandler.__init__` [bad-argument-type]
-  --> tests/unit_tests/core/callback_handler/test_index_tool_callback_handler.py:63:20
+  --> tests/unit_tests/core/callback_handler/test_index_tool_callback_handler.py:87:20
 ERROR Argument `None` is not assignable to parameter `message_id` with type `str` in function `core.callback_handler.index_tool_callback_handler.DatasetIndexToolCallbackHandler.__init__` [bad-argument-type]
-  --> tests/unit_tests/core/callback_handler/test_index_tool_callback_handler.py:64:24
+  --> tests/unit_tests/core/callback_handler/test_index_tool_callback_handler.py:88:24
 ERROR Argument `None` is not assignable to parameter `user_id` with type `str` in function `core.callback_handler.index_tool_callback_handler.DatasetIndexToolCallbackHandler.__init__` [bad-argument-type]
-  --> tests/unit_tests/core/callback_handler/test_index_tool_callback_handler.py:65:21
+  --> tests/unit_tests/core/callback_handler/test_index_tool_callback_handler.py:89:21
 ERROR Argument `None` is not assignable to parameter `invoke_from` with type `InvokeFrom` in function `core.callback_handler.index_tool_callback_handler.DatasetIndexToolCallbackHandler.__init__` [bad-argument-type]
-  --> tests/unit_tests/core/callback_handler/test_index_tool_callback_handler.py:66:25
+  --> tests/unit_tests/core/callback_handler/test_index_tool_callback_handler.py:90:25
 ERROR Argument `None` is not assignable to parameter `query` with type `str` in function `core.callback_handler.index_tool_callback_handler.DatasetIndexToolCallbackHandler.on_query` [bad-argument-type]
-  --> tests/unit_tests/core/callback_handler/test_index_tool_callback_handler.py:69:26
+  --> tests/unit_tests/core/callback_handler/test_index_tool_callback_handler.py:93:26
 ERROR Argument `None` is not assignable to parameter `dataset_id` with type `str` in function `core.callback_handler.index_tool_callback_handler.DatasetIndexToolCallbackHandler.on_query` [bad-argument-type]
-  --> tests/unit_tests/core/callback_handler/test_index_tool_callback_handler.py:69:32
+  --> tests/unit_tests/core/callback_handler/test_index_tool_callback_handler.py:93:32
 ERROR Argument `list[DummyToolInvokeMessage]` is not assignable to parameter `tool_outputs` with type `Iterable[ToolInvokeMessage]` in function `core.callback_handler.workflow_tool_callback_handler.DifyWorkflowCallbackHandler.on_tool_execution` [bad-argument-type]
   --> tests/unit_tests/core/callback_handler/test_workflow_tool_callback_handler.py:55:30
 ERROR Argument `list[DummyToolInvokeMessage]` is not assignable to parameter `tool_outputs` with type `Iterable[ToolInvokeMessage]` in function `core.callback_handler.workflow_tool_callback_handler.DifyWorkflowCallbackHandler.on_tool_execution` [bad-argument-type]

github-actions · 2026-06-30T13:47:23Z

Pyrefly Type Coverage

Metric	Base	PR	Delta
Type coverage	52.27%	52.27%	0.00%
Strict coverage	51.78%	51.78%	0.00%
Typed symbols	31,977	31,977	0
Untyped symbols	29,478	29,478	0
Modules	2969	2969	0

Copilot

Pull request overview

This PR fixes a transactional safety issue in the API by isolating “side-effect” ORM writes (audit logging, analytics counters, and message file persistence) from the shared Flask request-scoped db.session, preventing unintended commits/flushes of unrelated pending changes during multimodal streaming and RAG callbacks.

Changes:

Persist MessageFile in AppRunner._handle_multimodal_image_content() using an independent SQLAlchemy session (sessionmaker(...).begin()), instead of db.session.commit().
Persist DatasetQuery audit rows in DatasetIndexToolCallbackHandler.on_query() using an independent session, and update DocumentSegment.hit_count in on_tool_end() using an independent Session.
Update/add unit tests to assert the new session-isolation behavior (i.e., that db.session.commit() is not used for these side effects).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
api/core/app/apps/base_app_runner.py	Writes `MessageFile` via an independent session to avoid committing/closing the request-scoped session during multimodal streaming.
api/core/callback_handler/index_tool_callback_handler.py	Isolates audit log persistence and hit-count updates into independent sessions to avoid mutating caller transaction boundaries.
api/tests/unit_tests/core/app/apps/chat/test_base_app_runner_multimodal.py	Updates multimodal tests to validate isolated session usage (no `db.session.commit()` for side effects).
api/tests/unit_tests/core/callback_handler/test_index_tool_callback_handler.py	Updates callback handler tests to validate isolated session usage for audit/hit-count paths.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

FFXN · 2026-07-02T09:57:01Z

Hi, @agarwalpranav0711 please resolve the conflicts.

agarwalpranav0711 · 2026-07-02T13:16:30Z

On it, resolving now. Will push shortly.

…ct-session-writes # Conflicts: # api/core/callback_handler/index_tool_callback_handler.py # api/tests/unit_tests/core/callback_handler/test_index_tool_callback_handler.py

…ct-session-writes

agarwalpranav0711 · 2026-07-02T18:40:47Z

@FFXN Conflicts resolved and pushed. Also synced with the latest main (#38309) — the session-threading refactor there works cleanly alongside our independent-session fix with zero conflicts. All tests passing (1058/1058) across callback_handler, app/apps, rag/retrieval, and tools modules, plus lint and format checks clean. Ready for another look whenever you have time, thanks!

agarwalpranav0711 · 2026-07-03T09:39:08Z

@FFXN @laipz8200 @QuantumGhost Just checking in — this has been open
for a few days now with all CI checks passing and conflicts resolved.
Happy to make any changes needed. Let me know if there's anything
blocking review on your end.

agarwalpranav0711 requested review from QuantumGhost and laipz8200 as code owners June 30, 2026 09:32

dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jun 30, 2026

Merge origin/main into fix/isolate-side-effect-session-writes

c01951c

wylswz requested a review from Copilot July 2, 2026 05:22

Copilot started reviewing on behalf of wylswz July 2, 2026 05:23 View session

Copilot AI reviewed Jul 2, 2026

View reviewed changes

agarwalpranav0711 added 3 commits July 3, 2026 00:00

Merge remote-tracking branch 'origin/main' into fix/isolate-side-effe…

afa97e2

…ct-session-writes # Conflicts: # api/core/callback_handler/index_tool_callback_handler.py # api/tests/unit_tests/core/callback_handler/test_index_tool_callback_handler.py

Merge remote-tracking branch 'origin/main' into fix/isolate-side-effe…

c22a5f8

…ct-session-writes

Merge latest origin/main into fix/isolate-side-effect-session-writes

21d301e

Merge branch 'main' into fix/isolate-side-effect-session-writes

bb17440

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(api): isolate side-effect session writes in multimodal and RAG handlers#38210

fix(api): isolate side-effect session writes in multimodal and RAG handlers#38210
agarwalpranav0711 wants to merge 6 commits into
langgenius:mainfrom
agarwalpranav0711:fix/isolate-side-effect-session-writes

agarwalpranav0711 commented Jun 30, 2026

Uh oh!

github-actions Bot commented Jun 30, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 30, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

FFXN commented Jul 2, 2026

Uh oh!

agarwalpranav0711 commented Jul 2, 2026

Uh oh!

agarwalpranav0711 commented Jul 2, 2026

Uh oh!

agarwalpranav0711 commented Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

agarwalpranav0711 commented Jun 30, 2026

Summary

Screenshots

Checklist

Uh oh!

github-actions Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pyrefly Diff

Uh oh!

github-actions Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pyrefly Type Coverage

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

FFXN commented Jul 2, 2026

Uh oh!

agarwalpranav0711 commented Jul 2, 2026

Uh oh!

agarwalpranav0711 commented Jul 2, 2026

Uh oh!

agarwalpranav0711 commented Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Jun 30, 2026 •

edited

Loading

github-actions Bot commented Jun 30, 2026 •

edited

Loading